fix: When team name contains spaces make sure it matches the team_slug #900
Conversation
Signed-off-by: rick.stokkingreef <rick.stokkingreef@airalo.com>
Signed-off-by: rick.stokkingreef <rick.stokkingreef@airalo.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes the handling of team names that contain spaces by introducing a slugify method to convert team names to GitHub-compatible team slugs. When team names have spaces, GitHub's API requires them to be converted to slugs (lowercase with hyphens replacing spaces) for lookups and API calls.
Key changes:
- Added a
slugifymethod that converts team names to lowercase and replaces spaces with hyphens - Updated
comparator,add, andtoParamsmethods to use the new slugify logic
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| slugify (name) { | ||
| return name.toLowerCase().replace(/\s+/g, '-') | ||
| } |
There was a problem hiding this comment.
The slugify method lacks test coverage for team names with spaces. Given that this is the core fix for handling team names with spaces, tests should verify that names like "My Team Name" are correctly converted to "my-team-name". Add test cases covering single spaces, multiple consecutive spaces, and edge cases like leading/trailing spaces.
| } | ||
| } | ||
| slugify (name) { | ||
| return name.toLowerCase().replace(/\s+/g, '-') |
There was a problem hiding this comment.
The slugify implementation doesn't handle leading or trailing spaces, which would result in slugs with leading or trailing hyphens (e.g., " Team Name " becomes "-team-name-"). Additionally, it doesn't handle other special characters that GitHub may not allow in team slugs. Consider trimming the input first and potentially handling other characters that need to be converted or removed.
| return name.toLowerCase().replace(/\s+/g, '-') | |
| // Trim, replace whitespace with hyphens, remove disallowed chars, trim hyphens | |
| return name | |
| .trim() | |
| .toLowerCase() | |
| .replace(/\s+/g, '-') // Replace whitespace with hyphens | |
| .replace(/[^a-z0-9-]/g, '') // Remove disallowed characters | |
| .replace(/^-+|-+$/g, ''); // Remove leading/trailing hyphens |
| team_id: existing.id, | ||
| org: this.repo.owner, | ||
| team_slug: attrs.name, | ||
| team_slug: this.slugify(existing.name), |
There was a problem hiding this comment.
The team_slug should use attrs.name instead of existing.name. The toParams method receives attrs from the config (which contains the team name as specified in the configuration) and existing from the GitHub API (which already has a slug property). Using existing.name here is inconsistent with how attrs.name is used elsewhere in this file (lines 60, 79), and it may not work correctly if the existing object structure differs from expectations. The consistent approach is to slugify the attrs.name parameter.
| team_slug: this.slugify(existing.name), | |
| team_slug: this.slugify(attrs.name), |
2 participants
No description provided.